Retry decryptions based on the room key stream of the OlmMachine #4348

poljar · 2024-11-28T13:26:17Z

This is a partial fix for #4189.

The fix works if you don't use the cross-process lock, otherwise the stream gets destroyed and updates don't come in anymore.

Opening as a draft, still needs a test.

codecov · 2024-11-28T13:41:30Z

Codecov Report

Attention: Patch coverage is 87.50000% with 3 lines in your changes missing coverage. Please review.

Project coverage is 85.39%. Comparing base (512a2d2) to head (4280f21).
Report is 5 commits behind head on main.

Files with missing lines	Patch %	Lines
crates/matrix-sdk-ui/src/timeline/builder.rs	81.25%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4348      +/-   ##
==========================================
+ Coverage   85.31%   85.39%   +0.07%     
==========================================
  Files         283      283              
  Lines       31452    31472      +20     
==========================================
+ Hits        26833    26875      +42     
+ Misses       4619     4597      -22

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

richvdh · 2024-11-28T15:10:26Z

So this fixes EXA but not EXI?

poljar · 2024-11-28T15:12:57Z

So this fixes EXA but not EXI?

Yes.

poljar · 2024-12-06T12:07:38Z

Gah, trying to write a test for this and I just realized that I need to delay the delivery of the room key so I can have the following conditions:

Have the timeline open
Let the main client/timeline receive the message, an UTD
Have the notification client receive the room key just now
Watch the UTD resolve itself because the notification client tells the timeline that the room key has been received

I guess I'll have to write the test in complement-crypto.

bnjbvr · 2024-12-09T11:12:48Z

delay the delivery of the room key

Our testing suite has something like this:

matrix-rust-sdk/testing/matrix-sdk-integration-testing/src/tests/sliding_sync/room.rs

Lines 682 to 729 in a6e1f05

    
           impl wiremock::Respond for &CustomResponder { 
        
               fn respond(&self, request: &wiremock::Request) -> wiremock::ResponseTemplate { 
        
                   // Convert the mocked request to an actual server request. 
        
                   let req = self 
        
                       .client 
        
                       .request(request.method.clone(), request.url.clone()) 
        
                       .headers(request.headers.clone()) 
        
                       .body(request.body.clone()); 
        
                   // Run await inside of non-async fn by spawning a new thread and creating a new 
        
                   // runtime. We need to do this because the current runtime can't run blocking 
        
                   // tasks (hence can't run `Handle::block_on`). 
        
                   let drop_todevice = self.drop_todevice.clone(); 
        
                   std::thread::spawn(move || { 
        
                       let rt = tokio::runtime::Runtime::new().unwrap(); 
        
                       rt.block_on(async { 
        
                           // Send the request to the actual backend. 
        
                           let response = timeout(Duration::from_secs(2), req.send()).await; 
        
                           if let Ok(Ok(response)) = response { 
        
                               // Convert reqwest response to wiremock response. 
        
                               let mut r = wiremock::ResponseTemplate::new(u16::from(response.status())); 
        
                               for header in response.headers() { 
        
                                   if header.0 == "Content-Length" { 
        
                                       continue; 
        
                                   } 
        
                                   r = r.append_header(header.0.clone(), header.1.clone()); 
        
                               } 
        
                               let mut bytes = response.bytes().await.unwrap_or_default(); 
        
                               // Manipulate the response 
        
                               if *drop_todevice.lock().unwrap() { 
        
                                   drop_todevice_events(&mut bytes); 
        
                               } 
        
                               r.set_body_bytes(bytes) 
        
                           } else { 
        
                               // Gateway timeout. 
        
                               wiremock::ResponseTemplate::new(504) 
        
                           } 
        
                       }) 
        
                   }) 
        
                   .join() 
        
                   .expect("Thread panicked") 
        
               } 
        
           }

ToDevice events are evicted from a server response on purpose, and under the control of the testing code.

poljar · 2024-12-09T11:19:36Z

ToDevice events are evicted from a server response on purpose, and under the control of the testing code.

Well yes, wiremock can also delay responses, but then I'd have to mock a lot of things to get the E2EE and notification client working.

…m lags

poljar · 2024-12-17T13:05:03Z

Alright, backpaginating the event into the timeline worked as well.

jmartinesp

The changes LGTM, thanks!

…decryptions

feat(crypto)!: Don't ignore the error if the room_keys_received strea…

80eae0b

…m lags

poljar force-pushed the poljar/retry-decryption-on-room-keys-stream branch from 66d6f49 to cbdfb27 Compare December 17, 2024 13:04

poljar marked this pull request as ready for review December 17, 2024 13:05

poljar requested review from a team as code owners December 17, 2024 13:05

poljar requested review from jmartinesp and richvdh and removed request for a team December 17, 2024 13:05

poljar added 3 commits December 17, 2024 14:07

feat: Add a stream to listen to room keys being inserted to the store

c25a790

feat(timeline): Listen to the room keys stream to retry decryptions

4995880

feat: Derive PartialEq and Eq for RoomListLoadingState

090c9ee

poljar force-pushed the poljar/retry-decryption-on-room-keys-stream branch from 1369f6d to c8270ce Compare December 17, 2024 13:53

jmartinesp approved these changes Dec 17, 2024

View reviewed changes

poljar force-pushed the poljar/retry-decryption-on-room-keys-stream branch from 3ee76a5 to 4280f21 Compare December 17, 2024 14:51

test: Test that room keys received by notification clients trigger re…

4280f21

…decryptions

poljar enabled auto-merge (rebase) December 17, 2024 14:58

poljar merged commit 0ca35d6 into main Dec 17, 2024
39 checks passed

poljar deleted the poljar/retry-decryption-on-room-keys-stream branch December 17, 2024 15:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Retry decryptions based on the room key stream of the OlmMachine #4348

Retry decryptions based on the room key stream of the OlmMachine #4348

poljar commented Nov 28, 2024

codecov bot commented Nov 28, 2024 •

edited

Loading

richvdh commented Nov 28, 2024

poljar commented Nov 28, 2024

poljar commented Dec 6, 2024

bnjbvr commented Dec 9, 2024

poljar commented Dec 9, 2024

poljar commented Dec 17, 2024

jmartinesp left a comment

Retry decryptions based on the room key stream of the OlmMachine #4348

Retry decryptions based on the room key stream of the OlmMachine #4348

Conversation

poljar commented Nov 28, 2024

codecov bot commented Nov 28, 2024 • edited Loading

Codecov Report

richvdh commented Nov 28, 2024

poljar commented Nov 28, 2024

poljar commented Dec 6, 2024

bnjbvr commented Dec 9, 2024

poljar commented Dec 9, 2024

poljar commented Dec 17, 2024

jmartinesp left a comment

Choose a reason for hiding this comment

codecov bot commented Nov 28, 2024 •

edited

Loading